-
Notifications
You must be signed in to change notification settings - Fork 578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added support for combo from an array of struct #725
base: master
Are you sure you want to change the base?
added support for combo from an array of struct #725
Conversation
fixed pointer arithmetic warnings
Thanks a lot. Would you mind adding a small example in the overview demo? It serves as a good space to test the code and make sure it's all still functional. |
I added a small example as requested. |
…ature/combobox_for_struct
/* Combobox from array of struct */ | ||
static int sel=0; | ||
struct student { | ||
int id; | ||
char* name; | ||
int age; | ||
char* major; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May want to declare the struct outside of the code block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declare or define? What would be the reasoning g for this? I have definitely seen structure definitions with local scope.
But then again. I don't think that is a c89 capability.
demo/common/overview.c
Outdated
static struct student students[4] = { | ||
{0, "----", 0, "----"}, | ||
{1, "Mike", 20, "CS"}, | ||
{2, "Jim", 19, "Maths"}, | ||
{3, "Julia", 20, "Biology"} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe for static definitions, those may want to be outside as well. In addition, using this format may make it easier to read...
static struct student students[4] = {
{
.id = 0,
.name = "---"
.age = 0,
.major = "---"
},
...
In addition, what is the first ----
entry for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "----" string it was so there was a blank entry in the list. but i think i'll edit the function to check if the name is NULL, if so it could uses default value.
--- a/src/nuklear_combo.c
+++ b/src/nuklear_combo.c
@@ -1,6 +1,5 @@
#include "nuklear.h"
#include "nuklear_internal.h"
-
/* ==============================================================
*
* COMBO
@@ -845,6 +844,8 @@ NK_API int nk_combo_from_struct_array(struct nk_context *ctx, const void *items,
nk_layout_row_dynamic(ctx, (float)item_height, 1);
for (i = 0; i < count; ++i) {
name = *(char**)((char*)items + stride);
+ if (!name)
+ name = "";
if (nk_combo_item_label(ctx, name, NK_TEXT_LEFT))
selected = i;
items = (char*)items + item_sz;
just for clarification, when you say "those may want to be outside as well", you mean moving the array definition outside the overview function or the code block for the combo_from_struct_array example ? I'm not sure i get it correctly.
@@ -819,6 +819,41 @@ nk_combo_callback(struct nk_context *ctx, void(*item_getter)(void*, int, const c | |||
nk_combo_end(ctx); | |||
} return selected; | |||
} | |||
|
|||
|
|||
NK_API int nk_combo_from_struct_array(struct nk_context *ctx, const void *items, int count, int item_sz, int stride, int selected, int item_height, struct nk_vec2 size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some API documentation? Define each parameter, and give examples where possible. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure about the function name. Do we have examples of this pattern elsewhere through Nuklear that we could copy the naming convention from?
I do agree the function name might not be the best, i'll try to come up with a better suited one. for the documentation, do you prefer that i do it in the source file / header for nk_combo ? or as a comment in the overview.c file ? |
Definitely in the NK combo section. Preferably in the nuclear.H file, as the packer puts that first. when reading the single header, the public api prototypes are listed and documented near the top, then private library comes second; then definitions later. So placing docs above the definition will not be seen by future readers unless they explicitly scroll through 30k lines to find it. Also, overview.c is not sourced for documentation |
I'd even say in |
- Added documentation - Added check for NULL char pointer for the name
fixed comment style.
Unfortunately, the documentation format you have chosen has been recently deprecated. Take a look at the other header blocks in the most recent |
This PR adds a new combo function
nk_combo_from_struct_array
the objective is to be able to make combos from an array of structs (the only requirement is for the struct to have a char* ptr to hold the name)
parameters are the same as for the
nk_combo
function, with the addition of the size of the struct and a stride to themember to use for the combo entry.
example: